feat(agw): add implicit auditlog usage on agw by ricardosrib · Pull Request #209 · SAP/cloud-sdk-python

ricardosrib · 2026-07-06T18:18:54Z

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Implicit audit log emission in AgentGatewayClient - the client now automatically creates an AuditClient on initialization (using the LoB destination) and sends a DataAccess audit event after every successful list_mcp_tools() and call_mcp_tool() call. The tenant ID is read from the telemetry context var (get_tenant_id()).

Related Issue

N/A

Type of Change

Please check the relevant option:

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Code refactoring
Dependency update

How to Test

Create a AgentGatewayClient with a valid tenant_subdomain in an environment where the audit log destination is configured
Call list_mcp_tools() or call_mcp_tool() with a request context that has a tenant ID set
Verify a DataAccess audit event is emitted with channel_type="MCP", channel_id="agent-gateway", object_type="mcp-tool", and the correct tenant_id and object_id
For customer agents (no tenant_subdomain): verify no audit event is emitted and no error is raised
Run unit tests: uv run pytest tests/agentgateway/unit/test_agw_client.py

Checklist

Before submitting your PR, please review and check the following:

I have read the Contributing Guidelines
I have verified that my changes solve the issue
I have added/updated automated tests to cover my changes
All tests pass locally
I have verified that my code follows the Code Guidelines
I have updated documentation (if applicable)
I have added type hints for all public APIs
My code does not contain sensitive information (credentials, tokens, etc.)
I have followed Conventional Commits for commit messages

Breaking Changes

N/A

Additional Notes

N/A

BSD awk (macOS) requires the '/' inside a character class to be escaped as '\/'. GNU awk accepts either form. Discovered during live run on PR #209. Impact: check-testing-depth now returns valid JSON on macOS orchestrate runs; previously it silently emitted an awk error and aggregate.sh skipped the report.

Discovered during live run against PR #209. Multiple checks used the pattern: count=$(echo "$diff" | grep -E 'pattern' 2>/dev/null | wc -l | tr -d ' ') Under 'set -o pipefail', when grep finds zero matches it returns exit 1, which propagates through the pipe. Under 'set -e', the containing script exits silently — no stderr, no stdout, exit code 1. The 'aggregate.sh skips invalid JSON' safety net (added earlier) masked this by producing a summary that omitted the failing checks. Fix: wrap each grep-in-pipe with { grep … || true; } so zero-match returns success. Affects: - check-deps-supply.sh (count_lines helper) - check-pr-size.sh (count_lines helper + mods_count) - check-telemetry.sh (client_files, test_files, new_decorators) - check-testing-depth.sh (new_modules) - check-deletion-hygiene.sh (hits) Impact: live orchestrate.sh runs now emit valid JSON for all 20 checks instead of 15/20. No FP regression (all fixes preserve semantics). Discovered by: real dry-run on cloud-sdk-python PR #209 (Ricardo's agentgateway auditlog PR).

Discovered live on cloud-sdk-python PR #209 (Ricardo, agentgateway auditlog). The check was counting string occurrences from the on-disk AST — global to the file — then anchoring the finding to the first occurrence's line number. When a PR (a) shifts line numbers by adding upstream code and (b) does not touch any of the actual repeated-string lines, PY-CON-01 fires for tech debt that pre-existed the PR. Fix: read $ADDED_LINES_FILE (already exported by orchestrate.sh) inside check_con_01. Filter each literal's occurrence list to lines actually in the PR's added set. Only fire when ${filtered} is non-empty AND total occurrences (across file, unchanged) still >= threshold. Anchor line becomes the first added-line occurrence. Backward-compat: if ADDED_LINES_FILE is unset (bats tests exercising the lib directly), fall back to the legacy full-file behavior. Two new regression tests pin both branches (untouched pre-existing repetition = no fire; at least one occurrence on added line = fires with correct anchor). Bats: 81/81 green on both repos.

NicoleMGomes · 2026-07-09T17:16:53Z

Update user guide

…_mcp_tool

NicoleMGomes · 2026-07-15T18:49:00Z

+
+logger = logging.getLogger(__name__)
+
+MCP_TOOL_INVOKED = "MCP_TOOL_INVOKED"


Isn't better to have enum for it?

I think an enum would add no much value here because, differently from AuditLogMode where each value means a different behavior for implicit audit logging, these string constants are just labels — they have no meaning as a structured type event. Also, an enum would require .value at every call, adding noise for no benefit in this case.

NicoleMGomes · 2026-07-15T18:56:19Z

+MCP_TOOL_FAILED = "MCP_TOOL_FAILED"
+
+
+def create_audit_client(


Could we have it as a helper for internal usage and not specific to agw?

Created helper.py in core/auditlog_ng/ with the reusable parts: AuditLogMode, create_audit_client, and send_event.

Any module that wants implicit audit logging only needs to:

Call create_audit_client at client construction time

Declare AuditLogMode in its ClientConfig

Define its own event name constants and thin wrappers on top of send_event

The AGW-specific implicit_auditlog.py is now just the MCP_TOOL* constants and three public functions. The protobuf construction and mode/tenant handling are fully abstracted in core.

The same pattern can be reused by any module that wants to add implicit audit logging.

NicoleMGomes · 2026-07-16T20:36:56Z

+    "send_audit_event_failed",
+]
+
+MCP_TOOL_INVOKED = "MCP_TOOL_INVOKED"


I believe this file can be removed and we can have an enum for event and direct use the send_event. What do you think?

We could even consider a decorator for it

I think it's cleaner to call send_event directly and having an enum to pass the event type to it now makes sense to me. Already implemented it.

NicoleMGomes · 2026-07-16T20:38:51Z

                for tenant-scoped token exchange.
                TODO: This parameter's requirement is still being clarified with
                the IBD team and may be removed if unnecessary.
+            user_id: User identifier recorded in the audit event when an


We should retrieve it from user token instead of asking. You can check IAS module for it.

NicoleMGomes · 2026-07-16T20:41:53Z

@@ -0,0 +1,148 @@
+"""Audit log helper for custom events.


Maybe we can rename it to refer this is for implicit usage of auditlog cross modules

…ient with McpToolEvent enum

ricardosrib added 3 commits July 6, 2026 12:00

feat: add implicit audit log usage on agent gateway

bb614bd

create audit client on initialization time of the agw client

fa3c770

update unit tests

bd60cd3

ricardosrib requested a review from a team as a code owner July 6, 2026 18:18

ricardosrib changed the title ~~feat: add implicit auditlog usage on agw~~ feat(agw): add implicit auditlog usage on agw Jul 6, 2026

ricardosrib and others added 3 commits July 6, 2026 15:38

Merge branch 'main' into feat/add-implicit-auditlog-on-agw

74d50c6

version bump

f99f6b4

resolve pre-commit issue

5515bea

ricardosrib force-pushed the feat/add-implicit-auditlog-on-agw branch from a9e31e7 to 5515bea Compare July 6, 2026 19:02

ricardosrib and others added 3 commits July 7, 2026 15:15

move import to the top

8d5b08e

ruff format

02dc447

Merge branch 'main' into feat/add-implicit-auditlog-on-agw

5550854

tiagoek added the sdk-review: ⚠️ flagged label Jul 8, 2026

tiagoek removed the sdk-review: ⚠️ flagged label Jul 8, 2026

tiagoek added sdk-review: ❌ blocked sdk-review: ⚠️ flagged and removed sdk-review: ❌ blocked sdk-review: ⚠️ flagged labels Jul 8, 2026

NicoleMGomes reviewed Jul 9, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/agentgateway/agw_client.py Outdated

NicoleMGomes reviewed Jul 9, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/agentgateway/agw_client.py Outdated

NicoleMGomes reviewed Jul 9, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/agentgateway/agw_client.py Outdated

ricardosrib and others added 7 commits July 10, 2026 14:10

keep auditlogging only on call_mcp_tool()

c16ac44

add justifying comments to suppressed audit log exception

3c916a1

Merge branch 'main' into feat/add-implicit-auditlog-on-agw

9652628

fix ty type errors in create_audit_client and apply ruff format

d21d0e6

emit MCP_TOOL_INVOKED/COMPLETED/FAILED audit lifecycle events on call…

5f1b070

…_mcp_tool

Merge branch 'main' into feat/add-implicit-auditlog-on-agw

c880dba

ruff format

867afd5

ricardosrib force-pushed the feat/add-implicit-auditlog-on-agw branch from a2a5c6e to 867afd5 Compare July 13, 2026 17:20

ricardosrib added 2 commits July 13, 2026 15:08

simplify send auditlog logic

e9f42f2

apply ruff format

cec0904

ricardosrib requested review from NicoleMGomes and tiagoek July 13, 2026 18:46

NicoleMGomes reviewed Jul 15, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/agentgateway/config.py Outdated

NicoleMGomes reviewed Jul 15, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/agentgateway/_auditlog_helper.py Outdated

NicoleMGomes reviewed Jul 15, 2026

View reviewed changes

ricardosrib added 3 commits July 16, 2026 09:54

extract the resolve tenant logic to a separate method

069eac8

refactor implicit auditlog on agw to use generic core helpers

f305020

update user guide

051c5fe

ricardosrib requested a review from NicoleMGomes July 16, 2026 18:59

NicoleMGomes reviewed Jul 16, 2026

View reviewed changes

Comment thread src/sap_cloud_sdk/core/auditlog_ng/user-guide.md

NicoleMGomes reviewed Jul 16, 2026

View reviewed changes

ricardosrib added 5 commits July 17, 2026 11:44

remove _implicit_auditlog wrapper, call send_event directly on agw_cl…

dd2b7af

…ient with McpToolEvent enum

resolve user initiator ID from JWT token claim in audit events

89d113e

separate token resolving logic from getting user id from token

112e7d8

rollback minor change in list_mcp_tool

a622630

ruff format

3b5db8e

ricardosrib requested a review from NicoleMGomes July 17, 2026 19:24


		logger = logging.getLogger(__name__)

		MCP_TOOL_INVOKED = "MCP_TOOL_INVOKED"

		MCP_TOOL_FAILED = "MCP_TOOL_FAILED"


		def create_audit_client(

Uh oh!

Conversation

ricardosrib commented Jul 6, 2026

Description

Related Issue

Type of Change

How to Test

Checklist

Breaking Changes

Additional Notes

Uh oh!

Uh oh!

NicoleMGomes commented Jul 9, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ricardosrib Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ricardosrib Jul 16, 2026 •

edited

Loading